Skip to content

[CMake] Remove unused dependency on Foundation build directory #1260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

finagolfin
Copy link
Member

See if this helps with swiftlang/swift#79621

@finagolfin
Copy link
Member Author

@swift-ci test

@stmontgomery stmontgomery added this to the Swift 6.x (main) milestone Aug 11, 2025
@stmontgomery stmontgomery added build 🧱 Affects the project's build configuration or process android 🤖 Android support bug 🪲 Something isn't working linux 🐧 Linux support (all distros) labels Aug 11, 2025
@stmontgomery
Copy link
Contributor

I thought these linker flags were necessary, so if your investigation leads you to believe we should remove them, please elaborate since I don't quite understand all the factors involved here. I skimmed the linked Swift PR but it looks like it's gone in a few different directions so I'm unclear where things stand there, currently.

@finagolfin
Copy link
Member Author

My understanding is that these flags add a dependency to the CMake target, which is in the Foundation build directory, eg foundation-linux-x86_64. However, both build-script and build.ps1 install all prior corelibs to a separate single toolchain or SDK directory before Testing is built, so it isn't necessary to pass in the original build directories too.

Of course, this didn't break anything so it was fine so far, but my linked frontend pull enforces that the compiler looks in the passed in -sdk and the Windows CI builds and uses that SDK early, from the trunk stdlib build onwards, so these flags confuse it when it gets to building Testing:

"C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\build\\5\\bin\\swiftc.exe" -frontend -typecheck-module-from-interface T:/x86_64-unknown-windows-msvc/Testing/swift/Testing.swiftinterface -target x86_64-unknown-windows-msvc -disable-objc-interop -sdk "T:/Program Files/Swift/Platforms/Windows.platform/Developer/SDKs/Windows.sdk" -I "T:\\x86_64-unknown-windows-msvc\\Testing\\swift" -I "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\swift-testing\\Sources\\_TestingInternals\\include" -I "T:\\x86_64-unknown-windows-msvc\\Dispatch" -I "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\swift-corelibs-libdispatch" -I "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\swift-corelibs-libdispatch\\src" -I "T:\\x86_64-unknown-windows-msvc\\Dispatch\\src" -I "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\swift-corelibs-libdispatch\\src\\BlocksRuntime" -I "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\swift-foundation\\Sources\\_FoundationCShims\\include" -I "T:\\x86_64-unknown-windows-msvc\\DynamicFoundation\\swift" -I "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\swift-foundation-icu\\icuSources\\include" -I "T:\\x86_64-unknown-windows-msvc\\Dispatch\\src\\swift\\swift" -enable-library-evolution -gnone -module-link-name Testing -package-name org.swift.testing -swift-version 6 -O -D SWT_NO_FOUNDATION_FILE_COORDINATION -D SWT_NO_SNAPSHOT_TYPES -D Testing_EXPORTS -define-availability "_mangledTypeNameAPI:macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0" -define-availability "_uttypesAPI:macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0" -define-availability "_backtraceAsyncAPI:macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0" -define-availability "_clockAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0" -define-availability "_regexAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0" -define-availability "_swiftVersionAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0" -define-availability "_typedThrowsAPI:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0" -define-availability "_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0, visionOS 99.0" -require-explicit-sendable -enable-experimental-feature AccessLevelOnImport -enable-upcoming-feature ExistentialAny -enable-upcoming-feature InternalImportsByDefault -enable-upcoming-feature MemberImportVisibility -enable-upcoming-feature InferIsolatedConformances -load-plugin-library T:/x86_64-unknown-windows-msvc/TestingMacros/TestingMacros.dll -in-process-plugin-server-path "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\build\\5\\bin\\SwiftInProcPluginServer.dll" -plugin-path "C:\\Users\\swift-ci\\jenkins\\workspace\\swift-PR-windows\\build\\5\\bin" -Xcc -v -Xcc -fmodule-map-file=C:/Users/swift-ci/jenkins/workspace/swift-PR-windows/swift-foundation/Sources/_FoundationCShims/include/module.modulemap -Xcc -fmodule-map-file=C:/Users/swift-ci/jenkins/workspace/swift-PR-windows/swift-foundation-icu/icuSources/include/_foundation_unicode/module.modulemap -autolink-library oldnames -autolink-library msvcrt -Xcc -D_MT -Xcc -D_DLL -module-name Testing
T:\Program Files\Swift\Platforms\Windows.platform\Developer\SDKs\Windows.sdk\usr\lib\swift\_FoundationCShims\module.modulemap:1:8: error: redefinition of module '_FoundationCShims'
1 | module _FoundationCShims {
  |        `- error: redefinition of module '_FoundationCShims'
2 |     header "_FoundationCShims.h"
3 | 

C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift-foundation\Sources\_FoundationCShims\include\module.modulemap:1:8: note: previously defined here
1 | module _FoundationCShims {
  |        `- note: previously defined here
2 |     header "_FoundationCShims.h"
3 | 

T:\Program Files\Swift\Platforms\Windows.platform\Developer\SDKs\Windows.sdk\usr\lib\swift\_foundation_unicode\module.modulemap:1:8: error: redefinition of module '_FoundationICU'
  1 | module _FoundationICU {
    |        `- error: redefinition of module '_FoundationICU'
  2 |     header "translit.h"
  3 |     header "ustdio.h"

C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift-foundation-icu\icuSources\include\_foundation_unicode\module.modulemap:1:8: note: previously defined here
  1 | module _FoundationICU {
    |        `- note: previously defined here
  2 |     header "translit.h"
  3 |     header "ustdio.h"

Note how it gets confused between the SDK path and the module map files from the source directories that these CMake targets are transitively adding.

Basically, the Windows CI was mixing and matching files from the build directories, the source directories, and the final installed SDK directory to build each new core library like Foundation or XCTest. Once my frontend pull makes sure non-Darwin platforms use only the installed SDK directory, these flags broke the Windows CI, because it is the only platform CI that uses that SDK directory very early on with the -sdk flag.

I went ahead and got rid of it for linux and other build-script platforms too, as build-script also installs everything to a toolchain directory before building Testing, so these flags aren't needed, but they don't break the linux CI because it doesn't pass an explicit -sdk flag, like the Windows CI does.

@finagolfin
Copy link
Member Author

Good news is that previously failing command now passed on the Windows CI with my frontend pull, bad news is the CI now can't find Foundation.lib for a subsequent link command. I will iterate on this a bit more and let you all know when it works.

@grynspan
Copy link
Contributor

@compnerd Please review when @finagolfin is ready.

@grynspan grynspan requested a review from compnerd August 11, 2025 16:53
@finagolfin finagolfin added the windows 🪟 Windows support label Aug 11, 2025
@finagolfin
Copy link
Member Author

This passed the linux CI and has no effect on the mac CI, now to see if I can get it working on Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android 🤖 Android support bug 🪲 Something isn't working build 🧱 Affects the project's build configuration or process linux 🐧 Linux support (all distros) windows 🪟 Windows support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants